NEW: @W-19397375@: Preserve working folders on error - #355
Conversation
77f3d40 to
f983481
Compare
f983481 to
af6e283
Compare
| this.validateEnvironment(nodeVersion); | ||
| this.config = config; | ||
| this.tempFolder = new TempFolder(fileSystem); | ||
| /* istanbul ignore next */ |
There was a problem hiding this comment.
Could you clarify why this needed the ignore?
There was a problem hiding this comment.
The callback is really what's ignored. The test could never measure this because the node process is on exit when the call back is fired. So no way to cover this with a unit test. Demo (end-to-end) shows things working though.
| const describeOptions: engApi.DescribeOptions = { | ||
| workspace: engApiWorkspace, | ||
| workingFolder: await this.tempFolder.createSubfolder(describeWorkingFolderName, engineName), | ||
| workingFolder: workingFolder, |
There was a problem hiding this comment.
| workingFolder: workingFolder, | |
| workingFolder, |
There was a problem hiding this comment.
I actually find that this syntax is super confusing and it ties the variable name to the field name. So I personally choose not to use this type of syntax from javascript typically. Just a personal preference.
There was a problem hiding this comment.
For example, if I use my editor to rename a variable - it causes issues if I have "workingFolder" as both the fieldname and the variable. I might not want to change the field name... but may want to change the variable name and editors don't know this typically and get it wrong. Sometimes convenience and short code causes bugs... which is why I love static typing as well. :-)
| export interface TempFolder { | ||
| getPath(): Promise<string>; | ||
| createSubfolder(...subfolderPathSegs: string[]): Promise<string>; | ||
| export interface FileSystem { |
There was a problem hiding this comment.
This is good - I feel like it could use elevating out of utils.ts
There was a problem hiding this comment.
But it is an implementation detail utility in my opinion.
randi274
left a comment
randi274
left a comment
There was a problem hiding this comment.
This will be great to for us to add for some additional debugging support!
2 participants
No description provided.